Skip to content

Consistently use --locked for cargo commands - #7040

Merged
ytmimi merged 7 commits into
rust-lang:mainfrom
jieyouxu:jieyouxu/ci/lock
Sep 15, 2026
Merged

ytmimi merged 7 commits into
rust-lang:mainfrom
jieyouxu:jieyouxu/ci/lock

Conversation

@jieyouxu

@jieyouxu jieyouxu commented Aug 21, 2026 •

Copy link
Copy Markdown
Member

Summary

Helps with #7037 but does not completely adddress it:

  • We still download, build and run the ecosystem integration test repositories without --locked.

For cases modulo the ecosystem integration tests, use --locked consistently -- in CI, bootstrap, docs, test harness, so that we use the exact dependencies checked in via Cargo.lock lockfiles instead of floating resolution, where we risk being exposed to fresh supply-chain attacks1.

Reminded me to double-check after yesterday's supply-chain attack.

Footnotes

  1. Obviously this won't help with long-term supply-chain attacks that get into the lockfile, but defense-in-depth etc. ↩

@jieyouxu jieyouxu added S-waiting-on-review Status: awaiting review from the assignee but also interested parties. A-CI Area: CI A-test-suite Area: test suites A-docs Area: documentation labels Aug 21, 2026
@jieyouxu
jieyouxu marked this pull request as draft August 21, 2026 10:01
@rustbot rustbot added S-waiting-on-author Status: awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: awaiting review from the assignee but also interested parties. labels Aug 21, 2026
@jieyouxu

jieyouxu commented Aug 21, 2026 •

Copy link
Copy Markdown
Member Author

Oh that's interesting, right, we don't check in lockfiles for the integration crates (since we just clone the repos)

@ytmimi

ytmimi commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

We should also add --locked when building from source in the Diff-Check 😁

info!("Building rustfmt from source");
let Ok(_) = Command::new("cargo")
.current_dir(dir)
.args(["build", "-q", "--release", "--bin", "rustfmt"])
.output()
else {
return Err(CheckDiffError::FailedSourceBuild(
"Error building rustfmt from source",
));
};

Comment thread ci/src/build_and_test.rs

@jieyouxu jieyouxu Aug 24, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussion: I considered if we can use --locked for the ecosystem integration tests. My assessment is that we can't in the general case:

  • Some of these, like bitflags, are library crates that do not check in a lockfile. We can technically patch in a lockfile ourselves but that's a maintenance PITA.
  • We could consider using --locked iff the repo has a lockfile, but cf. no-lockfile case above.

View changes since the review

@jieyouxu

jieyouxu commented Aug 24, 2026 •

Copy link
Copy Markdown
Member Author

Changes since last review:

The ecosystem crate case feels a bit iffy to me, because that's a very large surface area versus rustfmt's own dependency graph. I think that will be helped once cargo stabilizes -Zmin-publish-age which might help us out here...

@rustbot review

@rustbot rustbot added S-waiting-on-review Status: awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: awaiting some action (such as code changes or more information) from the author. labels Aug 24, 2026
@jieyouxu
jieyouxu marked this pull request as ready for review August 24, 2026 09:41
@jieyouxu

Copy link
Copy Markdown
Member Author

r? ytmimi

@ytmimi
ytmimi added this pull request to the merge queue Sep 15, 2026
Merged via the queue into rust-lang:main with commit 8d0d374 Sep 15, 2026
27 checks passed
@rustbot rustbot added release-notes Needs an associated changelog entry and removed S-waiting-on-review Status: awaiting review from the assignee but also interested parties. labels Sep 15, 2026
@jieyouxu
jieyouxu deleted the jieyouxu/ci/lock branch September 16, 2026 00:17
@rustbot rustbot added this to the 1.100.0 milestone Sep 22, 2026
@ytmimi ytmimi removed the release-notes Needs an associated changelog entry label Sep 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-CI Area: CI A-docs Area: documentation A-test-suite Area: test suites

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants